refactor: avoid utils package name to fix revive issue #1895
+993
−990
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the generic pkg/utils tool-result helpers into a more explicit pkg/mcpresult package, then updates all call sites while keeping a thin deprecated compatibility layer for backwards compatibility.
Changes:
- Introduces
pkg/mcpresultwithNewText,NewError,NewErrorFromErr, andNewResourcehelpers wrapping MCP call-tool results. - Converts all existing users in
pkg/githubandpkg/errorsfromutils.NewToolResult*tomcpresult.*. - Leaves
pkg/utils/result.goas a deprecated shim that delegates tomcpresult, preserving the old API surface.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/utils/result.go | Turns utils result helpers into a deprecated shim that delegates to mcpresult, maintaining backward-compatible APIs. |
| pkg/mcpresult/mcpresult.go | Adds the new focused mcpresult package implementing the actual result-construction helpers used across the codebase. |
| pkg/github/server.go | Updates the JSON-marshalling helper to use mcpresult for success and error tool results. |
| pkg/github/security_advisories.go | Switches all advisory tools to construct error/success tool results via mcpresult. |
| pkg/github/secret_scanning.go | Routes secret scanning tools’ success and error responses through mcpresult. |
| pkg/github/search_utils.go | Uses mcpresult for search error handling and response marshalling. |
| pkg/github/search.go | Migrates repository/code/user/org search tools to the new mcpresult helpers. |
| pkg/github/repositories_test.go | Adjusts test expectations to use mcpresult.NewError rather than the deprecated utils helpers. |
| pkg/github/repositories_helper.go | Updates repository helper responses (match results and errors) to use mcpresult. |
| pkg/github/repositories.go | Replaces all repository tool result construction (commits, branches, files, tags, releases, stars) with mcpresult calls. |
| pkg/github/pullrequests.go | Refactors pull request tools (read, write, reviews, Copilot review) to use mcpresult for text and error responses. |
| pkg/github/projects.go | Updates projects tools (list/get/update/delete items, fields, discovery helpers) to rely on mcpresult for tool results. |
| pkg/github/notifications.go | Switches notification tools (list, dismiss, mark read, manage subscriptions) to mcpresult helpers. |
| pkg/github/labels.go | Adapts label read/write tools to construct responses via mcpresult. |
| pkg/github/issues.go | Uses mcpresult for all issue-related responses, including lockdown errors, sub-issues, comments, and issue create/update flows. |
| pkg/github/git.go | Changes repository tree tool result construction from utils to mcpresult. |
| pkg/github/gists.go | Migrates gist listing/reading/creating/updating tools to mcpresult. |
| pkg/github/feature_flags_test.go | Updates the test-only HelloWorldTool to use mcpresult in its test assertions. |
| pkg/github/dynamic_tools.go | Uses mcpresult for dynamic toolset enable/list helpers instead of utils. |
| pkg/github/discussions.go | Routes discussion tools (list, get, comments, categories) through mcpresult for text/error handling. |
| pkg/github/dependabot.go | Updates Dependabot alert tools to use mcpresult for results and error wrapping. |
| pkg/github/context_tools.go | Switches context tools (GetMe, teams, team members) to use mcpresult. |
| pkg/github/code_scanning.go | Refactors code scanning alert tools to use mcpresult helpers. |
| pkg/github/actions.go | Systematically replaces all Actions tools’ result construction (runs, jobs, logs, artifacts, reruns, cancellation) with mcpresult. |
| pkg/errors/error.go | Changes GitHub error helpers to construct MCP error results via mcpresult.NewErrorFromErr while preserving existing behavior. |
alexandear
force-pushed
the
refactor/rename-utils-mcpresult
branch
2 times, most recently
from
1e7807f to
5076b51
Compare
Collaborator
|
@alexandear am I missing something? The linting setup in CI doesn't seem to flag this, and if we are making lint fix PRs I would expect that they include enabling rules in CI etc. otherwise there is nothing to prevent regression. |
alexandear
commented
Jan 26, 2026
alexandear
force-pushed
the
refactor/rename-utils-mcpresult
branch
from
23b4bc9 to
f2deffa
Compare
alexandear
changed the title
alexandear
force-pushed
the
refactor/rename-utils-mcpresult
branch
from
f2deffa to
4dfcc2c
Compare
Contributor
Author
|
@SamMorrowDrums I have updated the PR and fixed merge conflicts. While it was under review, two more files were added to the
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Rename the
pkg/utilspackage topkg/mcpresultandpkg/githubapiwith clearer function names to resolve arevive.var-namingissue.Why
The
pkg/utilspackage name is "bad", triggering a revive linter warning. Renaming topkg/mcpresultandpkg/githubapimakes the purpose explicit and improves code clarity. Function names are simplified fromNewToolResultXtoNewXto align with Go naming conventions and reduce redundancy.What changed
pkg/mcpresultpackage with clearer function names:NewText,NewError,NewErrorFromErr,NewResourcepkg/githubapipackagemcpresultorgithubapidirectlyMCP impact
Prompts tested (tool changes only)
Security / limits
Tool renaming
deprecated_tool_aliases.goThis refactoring renames internal utility functions and packages, not MCP tools.
Note: if you're renaming tools, you must add the tool aliases. For more information on how to do so, please refer to the official docs.
Lint & tests
./script/lint./script/testDocs